Skip to content

Fletcher/parity testing#131

Open
FletcherDares wants to merge 5 commits into
mainfrom
fletcher/parity-testing
Open

Fletcher/parity testing#131
FletcherDares wants to merge 5 commits into
mainfrom
fletcher/parity-testing

Conversation

@FletcherDares
Copy link
Copy Markdown
Collaborator

No description provided.

@github-actions
Copy link
Copy Markdown

AI review done up to commit: 28cf9a1

AI Review Summary:

The pull request introduces significant enhancements to the datetime functions, aiming for better parity with SQLite's datetime modifiers. It also refactors the handling of SELECT statements without a FROM clause and adds a comprehensive SQLite parity testing framework.

Key Changes:

  • Datetime Functionality (julian_day.rs, mod.rs, modifiers.rs):

    • The JulianDay struct was simplified by removing the is_subsecond field, centralizing subsecond handling.
    • A new to_calendar_components method was introduced for robust date and time decomposition, including millisecond rounding for precision.
    • The DateTimeModifier enum was redesigned to explicitly represent various SQLite modifiers (e.g., AddYears, ShiftDate, StartOfMonth), moving away from a generic JDNOffset.
    • The parse_modifier function was rewritten to correctly parse a wider range of SQLite-compatible datetime modifier strings, including unit modifiers, date/time shifts (+YYYY-MM-DD HH:MM:SS), and special keywords.
    • Helper functions add_months and add_years were implemented to handle calendar arithmetic, including month/year rollovers and leap year considerations, ensuring accurate date calculations.
    • The weekday modifier logic was carefully implemented to match SQLite's behavior, advancing to the next occurrence of a target weekday.
  • SELECT Statement Parsing (select/mod.rs, select_statement.rs):

    • The SQL parser was updated to support SELECT statements that do not include a FROM clause (e.g., SELECT 1; or SELECT date('now');), by creating a temporary, empty table if no source table is specified.
  • SQLite Parity Testing (common/parity.rs, main_tests.rs, sqlite_parity.rs):

    • A new ParityManager was introduced to run SQL queries against both Mollycache and an in-memory SQLite database, comparing their results.
    • A values_equal_approx function was implemented to handle floating-point comparisons with an epsilon tolerance, crucial for comparing real numbers across different implementations.
    • New test suites were added to verify parity for basic math, datetime operations, and fundamental CRUD (Create, Read, Update, Delete) operations.

Overall Quality Assessment:

The changes represent a significant improvement in the functional correctness and robustness of the datetime features, bringing them much closer to SQLite's established behavior. The refactoring of the datetime modifier logic is a major step forward, making the system more modular and maintainable. The introduction of the SQLite parity testing framework is an excellent addition, providing a strong safety net for future development and ensuring high fidelity with a well-known SQL database. The code is well-structured, and the logical handling of complex date arithmetic and parsing has been carefully implemented.

Recurring Issues/Patterns:

No recurring issues or significant bugs were observed during the review. The changes consistently demonstrate a focus on correctness and alignment with SQLite's behavior. The datetime functionality is now much more comprehensive and reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant